CodeOwnersParser Team/User lookup #6366
Conversation
JimSuplizio
added
the
Central-EngSys
tools/code-owners-parser/CodeOwnersManualTester/CodeOwnerUtils.cs
Outdated
Show resolved
Hide resolved
tools/code-owners-parser/CodeOwnersManualTester/CodeOwnerUtils.cs
Outdated
Show resolved
Hide resolved
tools/code-owners-parser/CodeOwnersManualTester/CodeOwnersManualTester.csproj
Outdated
Show resolved
Hide resolved
| string ignoredPathPrefixes = DefaultIgnoredPrefixes) | ||
| string ignoredPathPrefixes = DefaultIgnoredPrefixes, | ||
| string? teamStorageURI = null, | ||
| string? ownersDataOutputFile = null) |
There was a problem hiding this comment.
@weshaggard, I had to add another command line option to RetrieveCodeOwners to allow writing the json output to the file. The fact is, the get-codeowners.lib.ps1 was incorrectly assuming any/all output from the executable was nothing more than a json codeowner entry. Using CodeOwnersParser FileHelper GetUrlContents to fetch the blob, will cause output that'll break this assumption. The correct fix for this is the add the option to write the entry to a file. The problem here is because of locked version in the eng/common/get-codeowners.lib.ps1 script, I can't make the changes, to use the file instead of output, to the script until this is checked in and another version has been published.
@konrad-jamrozik and I already chatted about this and both agreed this was the correct way to fix this.
| { | ||
| return list.ToDictionary((keyItem) => keyItem.Key, (valueItem) => valueItem.Value); | ||
| } | ||
| Console.WriteLine($"Error! Unable to deserialize json team/user data. rawJson={rawJson}"); |
There was a problem hiding this comment.
Probably worth including the TeamUserStorageURI as part of the error message as well.
There was a problem hiding this comment.
I'll add this in another commit before I merge.
| { | ||
| if (TeamUserDict.ContainsKey(teamWithoutOrg)) | ||
| { | ||
| Console.WriteLine($"Found team entry for {teamWithoutOrg}"); |
There was a problem hiding this comment.
We might want to think about making this logging configurable or use a logging interface at some point as generally it isn't great for shared libraries to have a bunch of logging.
There was a problem hiding this comment.
That won't be part of this PR. I created a new issue to track this.
When CodeOwnersParser encounters a team in the CODEOWNERS file it is parsing add the team's users, instead of the team, to the Owners list for the path. When adding the users for a team to the Owners list, it's done using a union to ensure that the resulting list is distinct. The list is pulled from blob data.
In the case where a team is encountered in the CODEOWNERS file that doesn't exist in the team/user data, the team will get added as an owner which is the behavior today. Because of this, the ExcludeNonUserAliases function still needs to be in there.
I'd also fixed an error I'd encountered with parsing where, if a line didn't have any owner entries, the owner was being set to the path expression. Here's what the CODEOWNERS line looks like when this occurs.
The other piece I've added to the CodeOwnersParser solution is a manual test project, CodeOwnersManualTester, which loads the selected CODEOWNERS files for testing/inspection purposes. Since this is not the default project for the solution, debugging just requires right clicking on the project and selecting Debug from there.